Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make com interfaces ide friendly #12201

Merged
merged 13 commits into from
Mar 25, 2021
Merged

Conversation

feerrenrut
Copy link
Contributor

@feerrenrut feerrenrut commented Mar 19, 2021

Link to issue number:

None
Related: #10916

Summary of the issue:

When working on modules that make heavy use of comInterfaces support from tools or IDEs would be helpful.
The files in comInterfaces are generated by comtypes.gen.
There are pairs of files, a module named with the comInterface ID, and one with a friendly name that imports symbols from the ID file.
The friendly name files generated by comtypes.gen expose symbols using runtime logic which some tools can not follow.
Making these symbols appear as errors/warnings about unknown symbols.

Description of how this pull request fixes the issue:

This PR modifies the build scripts to edit the friendly name file to include a fallback import statement which tools are more likely to be able to follow.

Essentially, the following:

from comtypes.gen import _C523F390_9C83_11D3_9094_00104BD0D535_0_3_0
globals().update(_C523F390_9C83_11D3_9094_00104BD0D535_0_3_0.__dict__)
__name__ = 'comtypes.gen.AcrobatAccessLib'

becomes:

try:
	from comtypes.gen import _C523F390_9C83_11D3_9094_00104BD0D535_0_3_0
except ModuleNotFoundError:
	import _C523F390_9C83_11D3_9094_00104BD0D535_0_3_0
	from _C523F390_9C83_11D3_9094_00104BD0D535_0_3_0 import *
globals().update(_C523F390_9C83_11D3_9094_00104BD0D535_0_3_0.__dict__)
__name__ = 'comtypes.gen.AcrobatAccessLib'

In addition, I noticed that Accessibility.py is generated, despite being commented out by the sconscript.
Presumably some other interface has a dependence on it?
Regardless this is used heavily in NVDA so it is now explicitly included.

The UIAutomationClient interface is not being generated, and instead was manually checked in.
This is because the version provided by Appveyor (and thus included in NVDA official builds) may not be as up to date as it could be. Instead, as new features of UIA are required this file must be manually updated by generating it on a machine that has the latest version available.

Testing strategy:

Built and ran locally.
Will test the appveyor build.
Ask community to test building and running on different versions of Windows.

Known issues with pull request:

None.

Change log entry:

Changes for devlopers:

- comInterfaces are now more easily consumable by developer tools such as IDEs. 

Code Review Checklist:

This checklist is a reminder of things commonly forgotten in a new PR.
Authors, please do a self-review and confirm you have considered the following items.
Mark items you have considered by checking them.
You can do this when editing the Pull request description with an x: [ ] becomes [x].
You can also check the checkboxes after the PR is created.

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

At runtime the import from comtypes.gen will work, as a fallback for IDE
/ Tools that can't find that the relative import is used.

Note: IDE's wont be aware of 'protected' symbols (beginning with
underscore), they aren't imported with the from x import * statment.
No functional changes.
Intended to make the following work easier to follow.
Generated "friedly name" comtypes.gen files are modified so that they
can be used by tools / IDEs more easily.

The files are read, the module name extracted, and the import statement
replaced by a more elaborate version which include a fall back to a do a
relative import if the comtypes.gen file is not found.
@AppVeyorBot

This comment has been minimized.

@LeonarddeR
Copy link
Collaborator

The UIAutomationClient interface was not being generated, and instead was manually checked in.

This was and is on purpose. The version of the com interface is tied to the operating system. If a newer version of the interface is released with a new version of Windows, it is usually not yet available in the appveyor image.

@josephsl
Copy link
Collaborator

josephsl commented Mar 19, 2021 via email

@lukaszgo1
Copy link
Contributor

Can't we use .idl files from Windows 10 SDK to generate UIAutomationClient COM interface similar to what were doing for IA2? We can require specific version of Windows 10 SDK and it isn't tied to the version of Windows in use.

@CyrilleB79
Copy link
Collaborator

Could this PR change the behaviour described in #10833 (scons --clean deletes some files that are fetched from the repo)?
Thanks.

@michaelDCurran
Copy link
Member

@feerrenrut yes several other typelibraries would have depended on the accessibility typelibrary. IAccessible2, and UI Automation being two of them.

Copy link
Member

@michaelDCurran michaelDCurran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't quite know what version of UIAutomationCore Appveyor has... I think its Windows server 2019 or something. Thus we cannot guarantee that UI Automation client will contain all possible interfaces and constants we need.
We need to either keep our pre-generated version of UIAutomationClient COM interfaces, or perhaps include uiAutomationCore.dll in miscDeps... though I think the latter would be breaking licensing restrictions.

source/comInterfaces_sconscript Outdated Show resolved Hide resolved
source/comInterfaces_sconscript Show resolved Hide resolved
@feerrenrut feerrenrut merged commit 8412adb into master Mar 25, 2021
@feerrenrut feerrenrut deleted the makeComInterfacesIDEfriendly branch March 25, 2021 07:03
@nvaccessAuto nvaccessAuto added this to the 2021.1 milestone Mar 25, 2021
michaelDCurran added a commit that referenced this pull request Apr 20, 2021
Fixes #12281

Math can no longer be read in NVDA with mathPlayer.
a regression introduced by pr #12201
comInterfaces_sconscript instructs comtypes to generate Python files from typelibs/mathPlayer.tlb. Comtypes produces MathPlayer.py (note the uppercase M). However, the sconscript target was hardcoded as "mathPlayer.py" (note the lowercase m). This has always been the case, and has not been a problem as scons and Windows is case insensitive and so Scons thought the target had been correctly created. However, pr #12201 makes use of the target's abspath property to rewrite the file with some extra imports. However, the path is made from the hardcoded target, so it has the lowercase m and therefore when the file is written out again, it has a lowercase m, and mathPres cannot then import MathPlayer with an uppercase M.

Description of how this pull request fixes the issue:
Correct the "mathPlayer.py" string in comInterfaces_sconscript to "MathPlayer.py".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants